Conversation
matrulda
left a comment
matrulda
left a comment
There was a problem hiding this comment.
Great work! I left some comments for you!
Other general stuff:
- Could you adapt the test scripts for py 3.13 and make sure we use that version in GHA?
- We use Nextflow 24.10.4 on Miarka, can you update the tests to use that version?
- It would be nice to check if we can update some of the singularity containers used: https://github.com/Molmed/seqreports/blob/main/config/nextflow_config/singularity.config. But it might be out of scope for this PR. We could make a separate ticket about that. Let me know what you think!
nkongenelly
force-pushed
the
DATAOPS-1297_support_for_bclconvert
branch
from
8b93c07 to
6654bd6
Compare
|
I have now updated the code after fixing things from the comments in the scripts. |
nkongenelly
force-pushed
the
DATAOPS-1297_support_for_bclconvert
branch
11 times, most recently
from
f886e82 to
cb7cc63
Compare
nkongenelly
force-pushed
the
DATAOPS-1297_support_for_bclconvert
branch
3 times, most recently
from
a244e6a to
d54947b
Compare
|
I have been unable to update GHA python version to v3.13 due to this error ' It also seems that there are issues with lxml with python v3.13 but i also tried with python v.3.12 and it failed with the same error in this run I also could NOT update checkQC version in requirements.txt to match the latest checkQC singularity container available i.e v4.0.7 as i got this error ERROR: No matching distribution found for checkqc==4.0.7 |
Did you try to update the requirements when using py3.11? The example you sent was with 3.13. |
matrulda
left a comment
matrulda
left a comment
There was a problem hiding this comment.
Awesome! A few final comments.
I think the unit tests are currently running with python v3.11 successfully now. Or might i have missed anything from your question? |
nkongenelly
force-pushed
the
DATAOPS-1297_support_for_bclconvert
branch
from
cfd8863 to
1ea3cba
Compare
|
Thanks for the reviews. I have now pushed the updated code |
Ah sorry for being unclear! I meant, were you able to update the dependencies (e.g. CheckQC) when using py3.11? |
Oooh. Okay. I have just tried it and it failed with the same error [here](ERROR: No matching distribution found for checkqc==4.0.7) |
nkongenelly
force-pushed
the
DATAOPS-1297_support_for_bclconvert
branch
2 times, most recently
from
d3c10a0 to
314d899
Compare
|
I have now pushed the updated code changing outdir default back to 'Unaligned' in the Comment |
Ah I see now, 4.07 (and 4.1.0) that is on pypi has "Requires: Python <3.11, >3.10". I think this was due to the old interop version that we used. I think 3.13 is supported now? 🤔 We should investigate this. |
I see that checkQC uses interop v1.4.0 which supports python v3.8-3.13 as seen here. Does this mean we might need to build checkqc with the latest python version first to pypi before using it here? |
yeah, I think we need create a new checkqc release and put it on pypi. I guess we could make it 4.2.0-rc1 perhaps? Or is it perhaps only a patch update 🤔 |
I think it can be just a patch update because BCLConvert was introduced in 4.1.0 bt not used yet... so maybe 4.1.1-rc1? |
New release candidate out on pypi: https://pypi.org/project/checkQC/4.1.1rc1/ You can try if it works better! :) |
Thanks, i've now updated the checkQC version in the requirements file |
2 participants
Description:
Risk Analysis:
This potentially could break report generation and cause a stop in production, but validation in staging env should catch serious bugs like that.
Validation Procedure:
This version might not be tested separately/individually as just this single seqreports service but as a whole by triggering the arteria workflows and looking at the multiqc reports generated